remove dead clear_partition for broadcast catalog stores#62
Merged
Conversation
Contributor
Author
Coordination with #61This PR conceptually depends on #61 (broadcast-catalog snapshot export) but the two are mergeable independently — no line-level overlap in the three shared files (
Auto-merge expected in either order. Version + CHANGELOGThis PR now carries the Recommended merge order: #61 first, then this PR. The CHANGELOG entry references #61 by number and reads naturally in that order. If #61 should carry the 0.3.5 bump instead: this PR rebases to 0.3.6 and the CHANGELOG entry becomes its own stanza. Trivial swap. |
fabracht
force-pushed
the
remove-broadcast-catalog-clear-partition
branch
from
3611fc5 to
518d4f8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #61. With schemas and constraints now treated as broadcast state on the export side (every partition snapshot carries the full catalog), the per-partition
clear_partitionmethods onSchemaStoreandConstraintStoreare wrong-by-construction — they let a partition wipe drop catalog entries that every node is supposed to hold.Audit finding before deleting:
StoreManager::clear_partition(the aggregator atpartition_io.rs:206-224) has zero callers anywhere in the codebase — not production, not tests. Pure dead code.SchemaStore::clear_partitionandConstraintStore::clear_partitionare referenced only by their own unit tests (clear_partition_removes_only_target). Nothing in the migration, rebalance, or snapshot paths calls them.import_partition(snapshot.rs:95) does not clear before importing, so the broadcast invariant established by bundle full schema and constraint catalogs in partition snapshots #61 is intact today.Rather than leaving a comment warning future readers not to wire these into a partition-relinquishment path, this PR removes the footgun outright:
StoreManager::clear_partition(15-line dead aggregator).SchemaStore::clear_partition+ its unit test.ConstraintStore::clear_partition+ its unit test.Per-partition
clear_partitiononDataStore,IndexStore,UniqueStore,FkStore, and the MQTT-side stores (sessions, retained, topics, etc.) is unchanged — those are genuinely partition-scoped.83 deletions, 0 additions, 3 files.
Test plan
cargo make clippy— clean (pedantic, all targets + wasm).cargo make format-check— clean.cargo make test— full workspace clean.grep -rn 'clear_partition'acrosscrates/confirms no residual references to the removed methods.Depends on
#61 — this PR is conceptually a follow-up; it makes sense regardless of merge order but the motivation comes from the broadcast-catalog model #61 establishes.